Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(compiler-cli): emit correct css string escape sequences #22776

Closed
wants to merge 3 commits into from

Conversation

chuckjaz
Copy link
Contributor

Works around an issue with TypeScript 2.6 and 2.7 that causes
the tranformer emit to emit incorrect escapes for css string
literals.

Fixes: #22774

PR Type

What kind of change does this PR introduce?

[x] Bugfix

What is the current behavior?

Issue Number: #22774

What is the new behavior?

Bypasses TypeScript incorrect string escaping to produce the correct .js file.

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Works around an issue with TypeScript 2.6 and 2.7 that causes
the tranformer emit to emit incorrect escapes for css string
literals.

Fixes: angular#22774
@chuckjaz chuckjaz requested a review from vicb March 14, 2018 20:38
@chuckjaz chuckjaz added the target: patch This PR is targeted for the next patch release label Mar 14, 2018
@mary-poppins
Copy link

You can preview 9942c35 at https://pr22776-9942c35.ngbuilds.io/.

Copy link
Contributor

@vicb vicb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Congrats on the terrible hack :)

@mary-poppins
Copy link

You can preview e6b004a at https://pr22776-e6b004a.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 752c819 at https://pr22776-752c819.ngbuilds.io/.

@mary-poppins
Copy link

You can preview e48718c at https://pr22776-e48718c.ngbuilds.io/.

@@ -150,13 +150,28 @@ function firstAfter<T>(a: T[], predicate: (value: T) => boolean) {
// NodeEmitterVisitor
type RecordedNode<T extends ts.Node = ts.Node> = (T & { __recorded: any; }) | null;

function escapeLiteral(value: string): string {
return value.replace(/(\"|\\)/g, '\\$1').replace(/(\n)|(\r)/g, function(v, n, r) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would .replace(\n).replace(\r) be more readable ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or .replace(/(\n|\r)/) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure but slower.

@kara kara added the area: core Issues related to the framework runtime label Mar 14, 2018
@chuckjaz chuckjaz added the action: merge The PR is ready for merge by the caretaker label Mar 15, 2018
mhevery pushed a commit that referenced this pull request Mar 15, 2018
Works around an issue with TypeScript 2.6 and 2.7 that causes
the tranformer emit to emit incorrect escapes for css string
literals.

Fixes: #22774

PR Close #22776
@mhevery mhevery closed this in 6e5e819 Mar 15, 2018
leo6104 pushed a commit to leo6104/angular that referenced this pull request Mar 25, 2018
…22776)

Works around an issue with TypeScript 2.6 and 2.7 that causes
the tranformer emit to emit incorrect escapes for css string
literals.

Fixes: angular#22774

PR Close angular#22776
thymikee pushed a commit to thymikee/jest-preset-angular that referenced this pull request Aug 4, 2018
- Upgrade example app to `Angular` v6.1 with `TypeScript` v2.9.
- Upgrade `jest` and `jest-preset-angular` to reflect the latest support of `jest` v23.
- Upgrade snapshot of `AppComponent` due to the [change](angular/angular#22776) in `Angular` v6.1.
- Upgrade other dependencies.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: core Issues related to the framework runtime cla: yes target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Literals in css with escapes are not emitted correctly
6 participants